feat: add inline @mention support to comment create#146
feat: add inline @mention support to comment create#146brigleb merged 2 commits intoneedmore:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds inline @mention support when creating comments, resolving @Name / @First.Last patterns against project members and emitting Basecamp <bc-attachment> tags so mentions render correctly in the Basecamp UI.
Changes:
- Captures
attachable_sgidonapi.Personso people can be referenced as Basecamp attachments. - Adds
UserResolver.ResolvePeopleto resolve identifiers to fullPersonobjects (not just IDs). - Updates
bc4 comment createto scan rich content for inline mentions and replace them with<bc-attachment>tags.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
internal/utils/users.go |
Adds ResolvePeople to return full Person objects for resolved identifiers. |
internal/api/client.go |
Extends Person with AttachableSGID to support mention attachment tags. |
cmd/comment/create.go |
Rewrites inline @... patterns in rich content into Basecamp attachment tags during comment creation. |
| // Replace inline @Name mentions with bc-attachment tags | ||
| // Supports @FirstName and @First.Last for disambiguation | ||
| inlineMatches := mentionRe.FindAllString(richContent, -1) | ||
| if len(inlineMatches) > 0 { | ||
| resolver := utils.NewUserResolver(client.Client, projectID) | ||
| // Convert @First.Last to "First Last" for resolution | ||
| identifiers := make([]string, len(inlineMatches)) | ||
| for i, m := range inlineMatches { | ||
| identifiers[i] = strings.ReplaceAll(strings.TrimPrefix(m, "@"), ".", " ") | ||
| } | ||
| people, err := resolver.ResolvePeople(f.Context(), identifiers) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to resolve mentions: %w", err) | ||
| } | ||
| for i, match := range inlineMatches { | ||
| tag := attachments.BuildTag(people[i].AttachableSGID) | ||
| richContent = strings.Replace(richContent, match, tag, 1) | ||
| } |
There was a problem hiding this comment.
The mention regexp will also match the "@Domain" part of email addresses (e.g., john@example.com -> @example.com) and potentially @... inside generated HTML attributes after markdown conversion. That can cause comment create to fail with "failed to resolve mentions" (or corrupt HTML by inserting <bc-attachment> into an attribute) when users include emails/URLs in the comment. Consider restricting matches to standalone mentions (start/whitespace boundary) and performing replacements only on HTML text nodes (e.g., parse with net/html and rewrite text nodes), or otherwise skipping matches that are part of an email/URL/link attribute.
| // ResolvePeople resolves a list of user identifiers to full Person objects | ||
| func (ur *UserResolver) ResolvePeople(ctx context.Context, identifiers []string) ([]api.Person, error) { | ||
| if err := ur.ensurePeopleCached(ctx); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| var people []api.Person | ||
| var notFound []string | ||
|
|
||
| for _, identifier := range identifiers { | ||
| identifier = strings.TrimSpace(identifier) | ||
| if identifier == "" { | ||
| continue | ||
| } | ||
|
|
||
| personID, found := ur.resolveIdentifier(identifier) | ||
| if !found { | ||
| notFound = append(notFound, identifier) | ||
| continue | ||
| } | ||
|
|
||
| for _, p := range ur.people { | ||
| if p.ID == personID { | ||
| people = append(people, p) | ||
| break | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if len(notFound) > 0 { | ||
| return nil, fmt.Errorf("could not find users: %s", strings.Join(notFound, ", ")) | ||
| } | ||
|
|
||
| return people, nil | ||
| } |
There was a problem hiding this comment.
ResolvePeople is new behavior but there are existing unit tests for UserResolver in internal/utils/users_test.go. Adding tests for this method (success cases, ordering, duplicates, and not-found/API-error behavior) would prevent regressions and ensure the returned Person objects (including AttachableSGID) are handled correctly.
- Require word boundary before @ to avoid matching inside emails/URLs - Treat empty identifiers as not-found in ResolvePeople for safe indexing - Add ResolvePeople tests covering happy path, errors, and edge cases Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use
@Nameor@First.Lastinline in comment content to mention people. The@reference is resolved against project members and replaced with a Basecamp<bc-attachment>tag, so it renders as a clickable mention in the UI.attachable_sgidfield toPersonstruct (already returned by Basecamp API, just wasn't captured)ResolvePeoplemethod toUserResolverthat returns fullPersonobjects (existingResolveUsersonly returns IDs)@wordpatterns after markdown conversion, resolve each to a person, replace with attachment tag